Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lingering progressbar text #81

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

lingering progressbar text #81

wants to merge 5 commits into from

Conversation

jspaaks
Copy link
Member

@jspaaks jspaaks commented Mar 11, 2021

Refs #80

output should now look like this (no lingering text)

┌─ 07:53:58 /tmp/demo.b347rJ (venv3) 
└─ $ fairtally `cat rsd-urls.txt | tail -n 2`
fairtally progress: |████████████████████████████████████████████| 2/2
currently checking https://github.com/puregome/notebooks    
┌─ 07:54:07 /tmp/demo.b347rJ (venv3)                                  <-- no lingering text here
└─ $

@jspaaks jspaaks marked this pull request as ready for review March 11, 2021 07:22
@sverhoeven
Copy link
Member

I tried to clear it as well in #48, but was not successful. This branch for my bash shell (on Linux Mint 20.1) looks like

clear;reset
(env) verhoes@slurper ~/git/fair-software/fairtally $ fairtally https://github.com/fair-software/fairtally
fairtally progress: |████████████████████████████████████████████| 1/1
Completed fairtally on 1 URLs and written report to tally.html
(env) verhoes@slurper ~/git/fair-software/fairtally $ lly.html

Having lly.html on the next prompt should not happen. When you press enter it does not execute ally.html so it is just a wrongly placed print.

sverhoeven
sverhoeven reviewed Mar 15, 2021
View reviewed changes
fairtally/cli.py Outdated Show resolved Hide resolved
@jspaaks jspaaks force-pushed the 80-lingering-progressbar-text branch from 889514d to e56710c Compare March 16, 2021 08:33
@jspaaks @sverhoeven
Update fairtally/cli.py
59c3009 
Co-authored-by: Stefan Verhoeven <[email protected]>
@jspaaks
Copy link
Member Author

jspaaks commented Mar 16, 2021

I fiddled with it some more, to no avail. I get the feeling we're missing something, but in the meantime, I've committed your suggestion.

sverhoeven
sverhoeven approved these changes Mar 16, 2021
View reviewed changes
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same feeling, something is not being reset/cleared correctly.

However this is better then before.

@jspaaks
Copy link
Member Author

jspaaks commented Mar 16, 2021

Now initializing the current_value helper bar with leave=False, and cleaning up the bar description with current_value.set_description_str(). This seems to get rid of the empty line I was getting in between the full progress bar and the 'Completed fairtally... ' line. Let me know what you think.

@jspaaks jspaaks requested a review from sverhoeven March 16, 2021 08:56
sverhoeven
sverhoeven requested changes Mar 16, 2021
View reviewed changes
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works for small URLs

(env) verhoes@slurper ~/git/fair-software/fairtally $ fairtally https://github.com/fair-software/fairtally
fairtally progress: |████████████████████████████████████████████| 1/1
Completed fairtally on 1 URLs and written report to tally.html
(env) verhoes@slurper ~/git/fair-software/fairtally $

However for long URL fails with URL not being cleaned up completely

(env) verhoes@slurper ~/git/fair-software/fairtally $ fairtally https://github.com/fair-software/fairtallygfdgdgdfgdfvbhslbvsljkhbjlhdbvjlhsd
fairtally progress: |████████████████████████████████████████████| 1/1
Completed fairtally on 1 URLs and written report to tally.htmlfdgdgdfgdfvbhslbvsljkhbjlhdbvjlhsd
(env) verhoes@slurper ~/git/fair-software/fairtally $

@jspaaks
Copy link
Member Author

jspaaks commented Mar 16, 2021

OK, I give up :( I'll revert the changes when I got a moment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sverhoeven sverhoeven sverhoeven requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jspaaks @sverhoeven